From 2d7a6c85c01b4d3402c94a2252d113079081ca99 Mon Sep 17 00:00:00 2001
From: "Carol (Nichols || Goulding)" <carol.nichols@gmail.com>
Date: Sat, 29 Aug 2015 23:17:31 -0400
Subject: [PATCH] Give better error messages for invalid manifest-paths

Inspired by https://github.com/rust-lang/cargo/pull/1182.
Fixes #1158, #1772, #1773.
---
 src/bin/read_manifest.rs          | 16 +---------------
 src/bin/verify_project.rs         |  9 +++++++++
 src/cargo/util/important_paths.rs | 11 ++++++++++-
 tests/test_cargo_read_manifest.rs |  8 ++++----
 4 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/src/bin/read_manifest.rs b/src/bin/read_manifest.rs
index 9c584fa2c..2539655f8 100644
--- a/src/bin/read_manifest.rs
+++ b/src/bin/read_manifest.rs
@@ -1,6 +1,5 @@
 use std::env;
 use std::error::Error;
-use std::path::PathBuf;
 
 use cargo::core::{Package, Source};
 use cargo::util::{CliResult, CliError, Config};
@@ -30,20 +29,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<Package>>
            env::args().collect::<Vec<_>>());
     try!(config.shell().set_color_config(options.flag_color.as_ref().map(|s| &s[..])));
 
-    // Accept paths to directories containing Cargo.toml for backwards compatibility.
-    let root = match options.flag_manifest_path {
-        Some(path) => {
-            let mut path = PathBuf::from(path);
-            if !path.ends_with("Cargo.toml") {
-                path.push("Cargo.toml");
-            }
-            Some(path.display().to_string())
-        },
-        None => None,
-    };
-    let root = try!(find_root_manifest_for_cwd(root));
-
-    debug!("read-manifest; manifest-path={}", root.display());
+    let root = try!(find_root_manifest_for_cwd(options.flag_manifest_path));
 
     let mut source = try!(PathSource::for_path(root.parent().unwrap(), config).map_err(|e| {
         CliError::new(e.description(), 1)
diff --git a/src/bin/verify_project.rs b/src/bin/verify_project.rs
index 86aa305a6..b1c1b4b61 100644
--- a/src/bin/verify_project.rs
+++ b/src/bin/verify_project.rs
@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use std::fs;
 use std::fs::File;
 use std::io::prelude::*;
 use std::process;
@@ -36,6 +37,14 @@ pub fn execute(args: Flags, config: &Config) -> CliResult<Option<Error>> {
 
     let mut contents = String::new();
     let filename = args.flag_manifest_path.unwrap_or("Cargo.toml".into());
+
+    if !filename.ends_with("Cargo.toml") {
+        fail("invalid", "the manifest-path must be a path to a Cargo.toml file")
+    }
+    if !fs::metadata(&filename).is_ok() {
+        fail("invalid", &format!("manifest path `{}` does not exist", filename))
+    }
+
     let file = File::open(&filename);
     match file.and_then(|mut f| f.read_to_string(&mut contents)) {
         Ok(_) => {},
diff --git a/src/cargo/util/important_paths.rs b/src/cargo/util/important_paths.rs
index 365ddabe3..1d6076b4b 100644
--- a/src/cargo/util/important_paths.rs
+++ b/src/cargo/util/important_paths.rs
@@ -41,7 +41,16 @@ pub fn find_root_manifest_for_cwd(manifest_path: Option<String>)
         human("Couldn't determine the current working directory")
     }));
     match manifest_path {
-        Some(path) => Ok(cwd.join(&path)),
+        Some(path) => {
+            let absolute_path = cwd.join(&path);
+            if !absolute_path.ends_with("Cargo.toml") {
+                return Err(human("the manifest-path must be a path to a Cargo.toml file"))
+            }
+            if !fs::metadata(&absolute_path).is_ok() {
+                return Err(human(format!("manifest path `{}` does not exist", path)))
+            }
+            Ok(absolute_path)
+        },
         None => find_project_manifest(&cwd, "Cargo.toml"),
     }
 }
diff --git a/tests/test_cargo_read_manifest.rs b/tests/test_cargo_read_manifest.rs
index 6f52bb636..52f79ccb9 100644
--- a/tests/test_cargo_read_manifest.rs
+++ b/tests/test_cargo_read_manifest.rs
@@ -51,8 +51,8 @@ test!(cargo_read_manifest_path_to_cargo_toml_parent_relative {
     assert_that(p.cargo_process("read-manifest")
                  .arg("--manifest-path").arg("foo")
                  .cwd(p.root().parent().unwrap()),
-                execs().with_status(0)
-                       .with_stdout(read_manifest_output()));
+                execs().with_status(101)
+                       .with_stderr("the manifest-path must be a path to a Cargo.toml file"));
 });
 
 test!(cargo_read_manifest_path_to_cargo_toml_parent_absolute {
@@ -63,8 +63,8 @@ test!(cargo_read_manifest_path_to_cargo_toml_parent_absolute {
     assert_that(p.cargo_process("read-manifest")
                  .arg("--manifest-path").arg(p.root())
                  .cwd(p.root().parent().unwrap()),
-                execs().with_status(0)
-                       .with_stdout(read_manifest_output()));
+                execs().with_status(101)
+                       .with_stderr("the manifest-path must be a path to a Cargo.toml file"));
 });
 
 test!(cargo_read_manifest_cwd {
-- 
2.30.2